Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: make "fs" service available in XBlock preview module system #610

Merged
merged 2 commits into from
Jan 17, 2024
Merged

fix: make "fs" service available in XBlock preview module system #610

merged 2 commits into from
Jan 17, 2024

Conversation

ArturGaspar
Copy link
Member

@ArturGaspar ArturGaspar commented Dec 6, 2023 •
edited
Loading

Description

The "fs" XBlock runtime service is not made available in Palm when in preview mode (e.g. in Studio).

This causes an error when trying to use this service while editing an XBlock, e.g. uploading a screenshot for the Recommender XBlock:

Traceback 
Traceback (most recent call last):
  File "/openedx/edx-platform/./cms/djangoapps/contentstore/views/preview.py", line 75, in preview_handler
    resp = instance.handle(handler, req, suffix)
  File "/openedx/venv/lib/python3.8/site-packages/xblock/mixins.py", line 84, in handle
    return self.runtime.handle(self, handler_name, request, suffix)
  File "/openedx/edx-platform/xmodule/x_module.py", line 1033, in handle
    return super().handle(block, handler_name, request, suffix=suffix)
  File "/openedx/venv/lib/python3.8/site-packages/xblock/runtime.py", line 1081, in handle
    results = handler(request, suffix)
  File "/openedx/venv/lib/python3.8/site-packages/recommender/recommender.py", line 581, in upload_screenshot
    fhwrite = self.fs.open(file_name, "wb")
  File "/openedx/venv/lib/python3.8/site-packages/xblock/reference/plugins.py", line 141, in __get__
    value = xblock.runtime.service(xblock, 'fs').load(self, xblock)
  File "/openedx/edx-platform/xmodule/x_module.py", line 1810, in service
    service = self._module_system.service(block, service_name)
  File "/openedx/edx-platform/xmodule/x_module.py", line 1740, in service
    service = super().service(block=block, service_name=service_name)
  File "/openedx/venv/lib/python3.8/site-packages/xblock/runtime.py", line 1125, in service
    raise NoSuchServiceError(f"Service {service_name!r} is not available.")
xblock.exceptions.NoSuchServiceError: Service 'fs' is not available.

This is fixed in master, possibly in 18fea86, however that is part of openedx#31472 which is a rather large change. This is a smaller fix.

Testing instructions

  1. Install and enable the Recommender XBlock

    • Install RecommenderXBlock from https://github.com/open-craft/RecommenderXBlock/tree/opencraft-release/2.1.0 if using S3
    • Make sure that openedx-django-pyfs is configured to use a writeable local directory, or an S3 bucket e.g. 
      # lms/envs/private.py
DJFS = {
    'type': 'osfs',
    'directory_root': 'lms/static/djpyfs',
    'url_root': '/static/djpyfs'
}
# cms/envs/private.py
DJFS = {
    'type': 'osfs',
    'directory_root': 'cms/static/djpyfs',
    'url_root': '/static/studio/djpyfs'
}
# Or,
DJFS = {
    'type': 's3fs',
    'bucket': AWS_STORAGE_BUCKET_NAME,
    'prefix': 'pyfs/',
    # Note that explicitly passing credentials requires openedx-django-pyfs >= 3.4.1
    'aws_access_key_id': AWS_ACCESS_KEY_ID,
    'aws_secret_access_key': AWS_SECRET_ACCESS_KEY
}

  2. In Studio, add a recommender component to a course.

  3. Add a resource in the recommender component, upload a preview screenshot.

    image

  4. Save it. Make sure there are no errors and that it was saved correctly.

  5. Reload the page. See the newly added item and that the screenshot loads on hover.

Deadline

None

Other information

Private-ref: https://tasks.opencraft.com/browse/BB-8077

@ArturGaspar ArturGaspar closed this Dec 7, 2023
@ArturGaspar ArturGaspar reopened this Dec 14, 2023
@ArturGaspar ArturGaspar reopened this Dec 19, 2023
@ArturGaspar ArturGaspar marked this pull request as ready for review December 19, 2023 22:26
@ArturGaspar
Copy link
Member Author

Need to merge open-craft/RecommenderXBlock#1 to fix the build for this.

viadanna
viadanna previously requested changes Dec 22, 2023
View reviewed changes
Copy link
Member

@viadanna viadanna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few small changes and we're good to go

requirements/edx/github.in Outdated

# https://github.com/openedx/django-pyfs/pull/169
git+https://github.com/open-craft/django-pyfs.git@opencraft-release/3.5.0#egg=openedx-django-pyfs==3.5.0
# https://github.com/openedx/RecommenderXBlock/pull/75
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I didn't mention this, but this is added via Grove, we don't need it in all our instances

requirements/edx/github.in Outdated
@@ -78,8 +78,10 @@
# Release candidates being tested.
##############################################################################

# ... add dependencies here

# https://github.com/openedx/django-pyfs/pull/169
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove the reference to openedx-django-pyfs in base.in?

Copy link
Member Author

@ArturGaspar ArturGaspar Dec 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@viadanna There is no direct reference to openedx-django-pyfs in base.in, it is pulled by the xblock requirement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, that's where we need to bump openedx-django-pyfs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@ArturGaspar ArturGaspar force-pushed the artur/xblock-preview-fs-service branch 2 times, most recently from 73ca602 to a3cd17a Compare December 27, 2023 21:05
@Cup0fCoffee
Copy link
Member

👍

  • I tested this
  • I read through the code
  • [n/a] I checked for accessibility issues
  • [n/a] Includes documentation
  • I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

@ArturGaspar ArturGaspar dismissed viadanna’s stale review January 17, 2024 08:06

Changes done but Paulo is on holidays.

@ArturGaspar ArturGaspar force-pushed the artur/xblock-preview-fs-service branch from a3cd17a to b726cd2 Compare January 17, 2024 08:07
Cup0fCoffee
Cup0fCoffee approved these changes Jan 17, 2024
View reviewed changes
Copy link
Member

@Cup0fCoffee Cup0fCoffee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ArturGaspar ArturGaspar merged commit bc82fe7 into opencraft-release/palm.1 Jan 17, 2024
41 checks passed
@ArturGaspar ArturGaspar deleted the artur/xblock-preview-fs-service branch January 17, 2024 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@viadanna viadanna viadanna left review comments

@Cup0fCoffee Cup0fCoffee Cup0fCoffee approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ArturGaspar @Cup0fCoffee @viadanna